Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap errors from external libraries to prevent leaking sensitive information #185

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

cheukwing
Copy link
Contributor

@cheukwing cheukwing commented Jul 20, 2022

Resolves #103

Introduce a SafeError type (credits to @alovak for the idea!) which is used to wrap around external errors, preventing the returned error message from displaying sensitive information, while still allowing errors to be matched.
Use this new error type to wrap external errors in the field and encoding packages, as these operate on the potentially sensitive data.

I considered also wrapping external errors in the prefix and network packages, but since these only operate on the length part of the data, exposing their details should be okay (as long as the message is correctly formatted).

@cheukwing cheukwing requested a review from alovak as a code owner July 20, 2022 14:08
@codecov-commenter
Copy link

Codecov Report

Merging #185 (dd4a723) into master (92f3b34) will increase coverage by 0.41%.
The diff coverage is 51.85%.

@@            Coverage Diff             @@
##           master     #185      +/-   ##
==========================================
+ Coverage   70.50%   70.91%   +0.41%     
==========================================
  Files          37       38       +1     
  Lines        1773     1798      +25     
==========================================
+ Hits         1250     1275      +25     
- Misses        334      336       +2     
+ Partials      189      187       -2     
Impacted Files Coverage Δ
field/ordered_map.go 88.88% <0.00%> (ø)
specs/builder.go 68.96% <0.00%> (ø)
field/binary.go 54.43% <28.57%> (-0.84%) ⬇️
field/composite.go 80.86% <33.33%> (-0.70%) ⬇️
field/string.go 75.34% <40.00%> (-1.81%) ⬇️
message.go 68.93% <40.00%> (-0.53%) ⬇️
field/numeric.go 79.74% <50.00%> (-1.84%) ⬇️
utils/safe_error.go 60.00% <60.00%> (ø)
encoding/hex.go 92.59% <75.00%> (+14.81%) ⬆️
encoding/ascii.go 100.00% <100.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92f3b34...dd4a723. Read the comment docs.

Copy link
Contributor

@Dakinola892 Dakinola892 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with all the new errors, especially the ones with expanded context - nice work

Copy link
Member

@adamdecaf adamdecaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a fan of this, nice!

Copy link
Contributor

@alovak alovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Thanks a lot!!!

@alovak alovak merged commit 151e28e into moov-io:master Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure sensitive information is not exposed through error messages
5 participants